Add --deps-only flag to separate dependency fetching from source builds by sbryngelson · Pull Request #1336 · MFlowCode/MFC

sbryngelson · 2026-03-25T14:52:27Z

Summary

Adds --deps-only CLI flag to ./mfc.sh build that only fetches and builds dependencies (FFTW, HDF5, etc.) without building MFC source targets
Already-configured dependencies are skipped during regular builds, preventing network access in the source build step
Frontier and Frontier AMD CI now fetch deps on login nodes (internet access) and build source on compute nodes (no network needed)

Test plan

Verify ./mfc.sh build --deps-only -j 8 builds only dependency targets
Verify regular ./mfc.sh build -j 8 skips already-configured deps
Verify Frontier CI: deps fetched on login node, source built + tested on compute node
Verify Frontier AMD CI: same pattern via symlinked build.sh
Verify Phoenix CI is unaffected (no login-node build step)
Verify bench.yml Frontier/Frontier AMD jobs work with updated build_script args

This allows CI to fetch and build dependencies (FFTW, HDF5, etc.) on login nodes with internet access, then build MFC source code on compute nodes that may have no network connectivity. Key changes: - New --deps-only CLI flag for ./mfc.sh build - Already-configured dependencies are skipped entirely during regular builds, guaranteeing no network access in the source build step - Frontier and Frontier AMD now follow the pattern: deps on login node, source build + test on compute node

coderabbitai · 2026-03-25T15:01:05Z

📝 Walkthrough

Walkthrough

This pull request refactors the CI/CD build workflows to separate dependency fetching from source building. Changes include removing the bench argument from Frontier build commands in the benchmarking workflow, updating workflow step labels from "Setup & Build" to "Fetch Dependencies" and "Test" to "Build & Test", removing conditional build-directory checks to enable unconditional builds, simplifying the Frontier build script to only fetch dependencies without the run_bench parameter, and adding a new --deps-only flag to the Python build CLI command that resolves and builds only dependencies without building MFC targets. The toolchain's build module now includes early-exit logic for already-configured dependencies and a dedicated control flow for the deps_only mode.

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	❓ Inconclusive	The PR description covers the main changes, motivation, and test plan, but is missing key required template sections like Type of change and Checklist items.	Complete the required template sections: specify Type of change (refactor, new feature, etc.), check the Checklist boxes for tests/documentation updates, and provide explicit testing details for unchecked items.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: adding a --deps-only flag to separate dependency fetching from source builds, which is the core feature across all modified files.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

codecov · 2026-03-25T17:48:19Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 47.42%. Comparing base (6120810) to head (7ab2744).
⚠️ Report is 4 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1336      +/-   ##
==========================================
+ Coverage   45.01%   47.42%   +2.41%     
==========================================
  Files          70       70              
  Lines       20562    19202    -1360     
  Branches     1962     1635     -327     
==========================================
- Hits         9255     9106     -149     
+ Misses      10179     9229     -950     
+ Partials     1128      867     -261

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

When __build_target recursively called build() to resolve sub-deps (e.g. SILO depends on HDF5), the --deps-only guard intercepted the recursive call and only built deps-of-deps (none for HDF5), never building HDF5 itself. This caused SILO's configure to fail with 'HDF5 was not found'. Fix: only enter the --deps-only path on the top-level call (history is empty), letting recursive calls proceed normally.

github-actions · 2026-03-25T17:52:56Z

Claude Code Review

Incremental review from: e836a1b
Head SHA: 7ab2744

New findings since last Claude review:

Frontier build parallelism regression (build.sh:43): The original test.sh used -j 1 for Frontier to work around a CCE 19.0.0 IPA SIGSEGV (comment: "Frontier Cray: -j 1 to work around CCE 19.0.0 IPA SIGSEGV"). The new build.sh unconditionally passes -j 8 for all clusters. If the CCE compiler bug is still present, Frontier builds will crash during parallel link/IPA, causing flaky CI.
--debug flag silently dropped for Frontier (test.sh:63): The old final test invocation included $debug_opts, which was --debug for Frontier ("for backtrace on build/runtime errors"). The new command drops it entirely. If intentional, a comment explaining the decision is absent; if accidental, Frontier runtime failures will lose backtraces.

Frontier/Frontier AMD now follow the same deps-on-login, source-on-compute pattern for case optimization tests. Previously, prebuild-case-optimization.sh built deps + source on the login node. Now: - Login node: build.sh fetches deps via --deps-only - Compute node: run_case_optimization.sh builds case-optimized binaries then runs them Phoenix is unchanged (prebuild + run both in SLURM).

…IGSEGV

…CCE 19 SIGSEGV

…g 7242

…se-opt builds Remove -h ipa0, --debug, and -j 1 workarounds for Frontier Cray builds. These did not address the root cause: corrupted intermediate files from prior failed builds persisting on self-hosted CI runners and poisoning subsequent compilations. Instead, clean stale hash-named MFC target staging/install dirs before case-opt builds while preserving dependency dirs (hipfort, fftw, etc.) that cannot be re-fetched without internet.

…d files Add clean: false to benchmark checkout steps, matching the test workflow. This prevents actions/checkout from failing on NFS silly-rename files (.nfs*) left by still-running SLURM processes, and preserves .slurm_job_id files so submit-slurm-job.sh can detect and cancel stale jobs on retry.

Split the combined Build & Test SLURM job into two sequential jobs so each gets its own full time allocation. This fixes timeouts on Frontier where build time (~20 min) was eating into the 1:59:00 test budget. Also removes Frontier -j 1/--debug workarounds from the build step.

sbryngelson marked this pull request as ready for review March 25, 2026 14:55

sbryngelson and others added 10 commits March 25, 2026 15:10

ci: use -j 1 for Frontier Cray builds to work around CCE 19.0.0 IPA S…

750b1fd

…IGSEGV

ci: suppress Cray warnings 990 and 7208

3adf7dc

ci: enable --debug for Frontier Cray builds for backtrace on errors

f42b7d5

ci: suppress Cray warning 7212; disable IPA in debug builds to avoid …

264d02c

…CCE 19 SIGSEGV

ci: move -h ipa0 to all Cray builds (not just debug); suppress warnin…

ba99b43

…g 7242

ci: replace deprecated Cray -G2 with -g -O0 for debug builds

2f1835a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add --deps-only flag to separate dependency fetching from source builds#1336

Add --deps-only flag to separate dependency fetching from source builds#1336
sbryngelson wants to merge 12 commits intoMFlowCode:masterfrom
sbryngelson:nodeps

sbryngelson commented Mar 25, 2026 •

edited

Loading

Uh oh!

coderabbitai bot commented Mar 25, 2026 •

edited

Loading

Walkthrough

❌ Failed checks (1 warning, 1 inconclusive)

Uh oh!

codecov bot commented Mar 25, 2026 •

edited

Loading

Uh oh!

github-actions bot commented Mar 25, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

1 participant

Conversation

sbryngelson commented Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

coderabbitai bot commented Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

❌ Failed checks (1 warning, 1 inconclusive)

Uh oh!

codecov bot commented Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions bot commented Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

1 participant

sbryngelson commented Mar 25, 2026 •

edited

Loading

coderabbitai bot commented Mar 25, 2026 •

edited

Loading

codecov bot commented Mar 25, 2026 •

edited

Loading

github-actions bot commented Mar 25, 2026 •

edited

Loading